-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[LLD][COFF] Add ignored linker flags #150815
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-lld-coff Author: None (kkent030315) ChangesAdded LLD flags to be ignored to align with MS link.exe.
Since LLVM does not implement POGO (Profile-guided Optimization, also known as "PGO"), this is completely ignorable without being problematic. Some of the flags are well described in this blog. Example of the errorLLD(lld-link.exe): 19.1.5 Full diff: https://github.com/llvm/llvm-project/pull/150815.diff 1 Files Affected:
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index 2a82fb5cd8845..bde1f5f950b4b 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -353,6 +353,18 @@ def fastfail : F<"fastfail">;
def kernel : F<"kernel">;
def pdbcompress : F<"pdbcompress">;
def emitpogophaseinfo : F<"emitpogophaseinfo">;
+def emittoolversioninfo: B<
+ "emittoolversioninfo",
+ "Emit a tool version info after DOS header (so-called Rich header, default)",
+ "Do not emit a tool version info after DOS header (so-scalled Ricb header)">;
+def novcfeature: B<
+ "novcfeature",
+ "Embeds tool version info in debug directory (default)",
+ "Do not embed tool version info in debug directory">;
+def nocoffgrpinfo: B<
+ "nocoffgrpinfo",
+ "Embeds COFFGRP info in debug directory (default)",
+ "Do not dmbeds COFFGRP info in debug directory">;
def noexp : F<"noexp">;
def delay : P_priv<"delay">;
|
|
@llvm/pr-subscribers-platform-windows Author: None (kkent030315) ChangesAdded LLD flags to be ignored to align with MS link.exe.
Since LLVM does not implement POGO (Profile-guided Optimization, also known as "PGO"), this is completely ignorable without being problematic. Some of the flags are well described in this blog. Example of the errorLLD(lld-link.exe): 19.1.5 Full diff: https://github.com/llvm/llvm-project/pull/150815.diff 1 Files Affected:
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index 2a82fb5cd8845..bde1f5f950b4b 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -353,6 +353,18 @@ def fastfail : F<"fastfail">;
def kernel : F<"kernel">;
def pdbcompress : F<"pdbcompress">;
def emitpogophaseinfo : F<"emitpogophaseinfo">;
+def emittoolversioninfo: B<
+ "emittoolversioninfo",
+ "Emit a tool version info after DOS header (so-called Rich header, default)",
+ "Do not emit a tool version info after DOS header (so-scalled Ricb header)">;
+def novcfeature: B<
+ "novcfeature",
+ "Embeds tool version info in debug directory (default)",
+ "Do not embed tool version info in debug directory">;
+def nocoffgrpinfo: B<
+ "nocoffgrpinfo",
+ "Embeds COFFGRP info in debug directory (default)",
+ "Do not dmbeds COFFGRP info in debug directory">;
def noexp : F<"noexp">;
def delay : P_priv<"delay">;
|
|
@llvm/pr-subscribers-lld Author: None (kkent030315) ChangesAdded LLD flags to be ignored to align with MS link.exe.
Since LLVM does not implement POGO (Profile-guided Optimization, also known as "PGO"), this is completely ignorable without being problematic. Some of the flags are well described in this blog. Example of the errorLLD(lld-link.exe): 19.1.5 Full diff: https://github.com/llvm/llvm-project/pull/150815.diff 1 Files Affected:
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index 2a82fb5cd8845..bde1f5f950b4b 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -353,6 +353,18 @@ def fastfail : F<"fastfail">;
def kernel : F<"kernel">;
def pdbcompress : F<"pdbcompress">;
def emitpogophaseinfo : F<"emitpogophaseinfo">;
+def emittoolversioninfo: B<
+ "emittoolversioninfo",
+ "Emit a tool version info after DOS header (so-called Rich header, default)",
+ "Do not emit a tool version info after DOS header (so-scalled Ricb header)">;
+def novcfeature: B<
+ "novcfeature",
+ "Embeds tool version info in debug directory (default)",
+ "Do not embed tool version info in debug directory">;
+def nocoffgrpinfo: B<
+ "nocoffgrpinfo",
+ "Embeds COFFGRP info in debug directory (default)",
+ "Do not dmbeds COFFGRP info in debug directory">;
def noexp : F<"noexp">;
def delay : P_priv<"delay">;
|
e2b8233 to
524c5fa
Compare
524c5fa to
0722bcb
Compare
This seems mildly incorrect - LLVM/Clang does support PGO just fine, but it doesn't support the MSVC intermediate files or produce the exact same metadata in the output as MSVC does. |
mstorsjo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change itself looks fine, but the commit message (the PR description) may need a little bit of rewording.
|
@mstorsjo Thanks for the review. I updated the PR title, does it look like good for you or should I fix something? Let me know :) |
mstorsjo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks! Although it could be good to clarify that those options affect metadata generation for the MSVC style PGO, and LLD doesn't do that. As it stands right now it is very unclear what these options have to do with the statement about PGO support, as the option names don't make that clear at all. So perhaps instead of this:
We'd have this:
Then also on another hand; the latter two options are undocumented, right? So there's quite little risk of them being used much in the wild at all (contrary to the first one which is totally understandable to handle). |
Updated. Thanks for the better one!
Well, there are many projects uses those flags (e.g., UEFI binaries) and I experienced the odd. For example, if I had this cmake setup, it is no way compatible with MSVC toolchain ( target_link_options(foo PRIVATE
# Decrease the binary size
/EMITTOOLVERSIONINFO:NO
/EMITPOGOPHASEINFO
/NOVCFEATURE
/NOCOFFGRPINFO
) |
Ok, fair enough. Thanks! Then I think this one looks good. |
These flags were treated as explicit errors, which could cause compatibility issues with MS link.exe. To address this, LLD was updated to ignore them, aligning its behavior with MS link.exe. The latter two options affect how MS link.exe generates metadata for its PGO. LLD doesn't generate any such metadata right now (and doesn't work with the MSVC PGO feature), so those options are kept as no-ops.
/emittoolversioninfo[:no]flag in LLD/COFF options/novcfeatureflag in LLD/COFF options/nocoffgrpinfoflag in LLD/COFF options